Skip to content

improve: naming on ResourceID API#3151

Closed
csviri wants to merge 1 commit intooperator-framework:nextfrom
csviri:resourceid-api
Closed

improve: naming on ResourceID API#3151
csviri wants to merge 1 commit intooperator-framework:nextfrom
csviri:resourceid-api

Conversation

@csviri
Copy link
Collaborator

@csviri csviri commented Feb 4, 2026

Signed-off-by: Attila Mészáros a_meszaros@apple.com

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
@openshift-ci openshift-ci bot requested review from metacosm and xstefank February 4, 2026 07:54
* @since 5.3.0
*/
public boolean isSameResource(String name, String namespace) {
public boolean equals(String name, String namespace) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that equals is a better name (because equals has a special meaning in Java and it doesn't quite match here), though I agree that isSameResource is not great either… maybe equilaventTo? This way, it would be somewhat discoverable if someone is looking for equality-like methods?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I though overloaded would be too much an issue, but you might be right that naming wise this could raise some eyebrows.

return Objects.equals(name, that.name) && Objects.equals(namespace, that.namespace);
}

public boolean isSameResource(HasMetadata hasMetadata) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would technically be an API break.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is true, so not sure to how extent we consider this as an internal API, but since users use this with customt SecondaryToPrimaryMapper it might be an issue.

@csviri
Copy link
Collaborator Author

csviri commented Feb 4, 2026

Ok, considering this we probably don't want to break this API will close this.

@csviri csviri closed this Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants